Feature/add null alt text to several images #9296

JanhviHarwani · 2024-05-21T19:26:21Z

Testing done

Confirmed the memory monitor text is visible from /extensionList/hudson.diagnosis.MemoryUsageMonitor/0/ and also confirmed that the graph does not render on that page

Proposed changelog entries

Add alternate text for images to help visually impaired Jenkins users.

Proposed upgrade guidelines

N/A

Submitter checklist

Give feedback

The Jira issue, if it exists, is well-described.
The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
There is automated testing or an explanation as to why this change has no tests.
New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
For dependency updates, there are links to external changelogs and, if possible, full differentials.
For new APIs and extension points, there is a link to at least one consumer.
Options

Desired reviewers

@mention

Before the changes are marked as ready-for-merge:

Maintainer checklist

Give feedback

There are at least two (2) approvals for the pull request and no outstanding requests for change.
Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
Proper changelog labels are set so that the changelog can be generated automatically.
If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).
Options

Open Issue: https://issues.jenkins.io/plugins/servlet/mobile#issue/JENKINS-62424
resolved: added null text to alt attribute to img tags wherever necessary

merging current changes

welcome · 2024-05-21T19:26:25Z

Yay, your first pull request towards Jenkins core was created successfully! Thank you so much!

A contributor will provide feedback soon. Meanwhile, you can join the chats and community forums to connect with other Jenkins users, developers, and maintainers.

MarkEWaite · 2024-05-21T20:15:46Z

@JanhviHarwani I'm leaving this pull request open because it might address the Jira issue that is mentioned. It does not complete the pull request template. It does not provide a description of the testing that was performed to assure that the pull request works as expected. It does not include a usable changelog entry.

Could you complete those items on the pull request?

We're happy to have more contributors to Jenkins, but we need those new contributors to follow the processes that we've outlined so that we don't waste the time of maintainers as they process pull requests that do not provide the required information.

MarkEWaite

Thanks for the proposals. I've suggested different text based on the context of the images.

MarkEWaite · 2024-05-21T20:16:47Z

.github/config.yml

@@ -20,6 +20,6 @@ firstPRMergeComment: >

  <a href="https://www.jenkins.io/participate/" target="_blank">
   <picture>
-     <img width="600" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">
+     <img width="600" alt="jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">


Jenkins is an English language proper noun. It needs to be capitalized.

Suggested change

<img width="600" alt="jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

<img width="600" alt="Jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

~~Needs to be localizable.~~

Suggested change

<img width="600" alt="jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

<img width="600" alt="${%Jenkins welcome page}" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

Additionally I am not sure welcome page adequately describes what this is.

I don't think that we can localize the text in pages that are generated by GitHub Actions.

Good point, I managed to comment this on the only file it doesn't apply to 😅 (all your suggestions are not localizable).

MarkEWaite · 2024-05-21T20:18:40Z

core/src/main/resources/hudson/diagnosis/MemoryUsageMonitor/index.jelly

@@ -61,7 +61,7 @@ THE SOFTWARE.
          </j:otherwise>
        </j:choose>
      </div>
-      <img src="graph?type=${type}&amp;width=500&amp;height=300" />
+      <img src="graph?type=${type}&amp;width=500&amp;height=300" alt="null" />


Since this is the memory usage monitor graph, I think that the alt text should say that. The word "null" won't help a disabled user understand what is being shown in the image.

Suggested change

<img src="graph?type=${type}&width=500&height=300" alt="null" />

<img src="graph?type=${type}&width=500&height=300" alt="memory usage monitor graph" />

Resolved in 904cb3e

core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly

core/src/main/resources/jenkins/model/Jenkins/_404.jelly

core/src/main/resources/jenkins/model/Jenkins/_404_simple.jelly

MarkEWaite · 2024-05-21T20:24:32Z

core/src/main/resources/jenkins/model/Jenkins/oops.jelly

@@ -32,7 +32,7 @@ THE SOFTWARE.
    <l:header />
    <l:main-panel>
        <h1 style="text-align: center">
-          <img src="${imagesURL}/rage.svg" height="179" width="154"/> <span style="font-size:50px"><st:nbsp/>${%Oops!}</span>
+          <img src="${imagesURL}/rage.svg" height="179" width="154" alt="null"/> <span style="font-size:50px"><st:nbsp/>${%Oops!}</span>


Suggested change

<img src="${imagesURL}/rage.svg" height="179" width="154" alt="null"/> <st:nbsp/>${%Oops!}

<img src="${imagesURL}/rage.svg" height="179" width="154" alt="Oops"/> <st:nbsp/>${%Oops!}

Decorative only. Also, null isn't intended as a literal value, but to represent the explicitly specified empty value (alt="").

Suggested change

<img src="${imagesURL}/rage.svg" height="179" width="154" alt="null"/> <st:nbsp/>${%Oops!}

<img src="${imagesURL}/rage.svg" height="179" width="154" alt=""/> <st:nbsp/>${%Oops!}

Resolved in 4d8fd42

core/src/main/resources/lib/hudson/executors.jelly

core/src/main/resources/lib/layout/dropdowns/custom.jelly

daniel-beck

This PR misunderstands the recommendation in Jira. null isn't intended as a literal value, but to represent the explicitly specified empty value (alt="").

daniel-beck · 2024-05-21T20:30:08Z

.github/config.yml

@@ -20,6 +20,6 @@ firstPRMergeComment: >

  <a href="https://www.jenkins.io/participate/" target="_blank">
   <picture>
-     <img width="600" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">
+     <img width="600" alt="jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">


~~Needs to be localizable.~~

Suggested change

<img width="600" alt="jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

<img width="600" alt="${%Jenkins welcome page}" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

Additionally I am not sure welcome page adequately describes what this is.

daniel-beck · 2024-05-21T20:31:36Z

core/src/main/resources/jenkins/model/Jenkins/oops.jelly

@@ -32,7 +32,7 @@ THE SOFTWARE.
    <l:header />
    <l:main-panel>
        <h1 style="text-align: center">
-          <img src="${imagesURL}/rage.svg" height="179" width="154"/> <span style="font-size:50px"><st:nbsp/>${%Oops!}</span>
+          <img src="${imagesURL}/rage.svg" height="179" width="154" alt="null"/> <span style="font-size:50px"><st:nbsp/>${%Oops!}</span>


Decorative only. Also, null isn't intended as a literal value, but to represent the explicitly specified empty value (alt="").

Suggested change

<img src="${imagesURL}/rage.svg" height="179" width="154" alt="null"/> <st:nbsp/>${%Oops!}

<img src="${imagesURL}/rage.svg" height="179" width="154" alt=""/> <st:nbsp/>${%Oops!}

merge jenkins master branch to feature/add-null-text

…o feature/add-null-text-alt

merging to local branch

…Harwani/jenkins into feature/add-null-text-alt

…tors.jelly

JanhviHarwani · 2024-05-24T02:02:39Z

I have resolved all the comments in the PR. Thank you for taking out time to look at the PR.

daniel-beck · 2024-05-24T09:52:31Z

.github/config.yml

@@ -20,6 +20,6 @@ firstPRMergeComment: >

  <a href="https://www.jenkins.io/participate/" target="_blank">
   <picture>
-     <img width="600" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">
+     <img width="600" alt=${%Jenkins welcome page} src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">


This applied the wrong suggestion from the comment thread. Mark pointed out I was wrong. (Also would be syntactically invalid in Jelly.)

Suggested change

<img width="600" alt=${%Jenkins welcome page} src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

<img width="600" alt="Jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

(Also I'm not sure this particular alt text is good, but that's a separate problem. I encourage you to consider what the image represents on this page, and how that would be accomplished in alt text.)

Alt text updated in 87d4ab4

daniel-beck · 2024-05-24T09:54:22Z

core/src/main/resources/hudson/diagnosis/MemoryUsageMonitor/index.jelly

@@ -61,7 +61,7 @@ THE SOFTWARE.
          </j:otherwise>
        </j:choose>
      </div>
-      <img src="graph?type=${type}&amp;width=500&amp;height=300" />
+      <img src="graph?type=${type}&amp;width=500&amp;height=300" alt="memory usage monitor graph" />


Suggested change

<img src="graph?type=${type}&width=500&height=300" alt="memory usage monitor graph" />

<img src="graph?type=${type}&width=500&height=300" alt="${%memory usage monitor graph}" />

The same applies to all .jelly files. (Previously mentioned in #9296 (comment))

Fixed in 904cb3e

daniel-beck · 2024-05-24T09:54:57Z

core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly

@@ -50,13 +50,13 @@ THE SOFTWARE.
            ${%No files in directory}
           <j:if test="${showSymlinkWarning}">
             <p>
-                <img id="symlinkalert" src="${imagesURL}/svgs/warning.svg" width="16" height="16" />
+                <img id="symlinkalert" src="${imagesURL}/svgs/warning.svg" width="16" height="16" alt="symlink alert"/>


Since the text in the next line exists, there is no need for an alt text here. This icon exists only to make the notice stand out.

Resolved in 8d861dd

daniel-beck · 2024-05-24T09:55:01Z

core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly

               ${%Symlinks are hidden}
             </p>
            </j:if>
           <j:if test="${showTmpDirWarning}">
             <p>
-               <img id="tmpdiralert" src="${imagesURL}/svgs/warning.svg" width="16" height="16" />
+               <img id="tmpdiralert" src="${imagesURL}/svgs/warning.svg" width="16" height="16" alt="temporary directory warning"/>


Since the text in the next line exists, there is no need for an alt text here. This icon exists only to make the notice stand out.

Resolved in 8d861dd

daniel-beck · 2024-05-24T09:55:06Z

core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly

@@ -124,13 +124,13 @@ THE SOFTWARE.
           </table>
           <j:if test="${showSymlinkWarning}">
             <p>
-                <img id="alert" src="${imagesURL}/svgs/warning.svg" width="16" height="16" />
+                <img id="alert" src="${imagesURL}/svgs/warning.svg" width="16" height="16" alt="symlink warning"/>


Since the text in the next line exists, there is no need for an alt text here. This icon exists only to make the notice stand out.

Resolved in 8d861dd

daniel-beck · 2024-05-24T09:55:09Z

core/src/main/resources/hudson/model/DirectoryBrowserSupport/dir.jelly

               ${%Symlinks are hidden}
             </p>
            </j:if>
           <j:if test="${showTmpDirWarning}">
             <p>
-                <img id="alert" src="${imagesURL}/svgs/warning.svg" width="16" height="16" />
+                <img id="alert" src="${imagesURL}/svgs/warning.svg" width="16" height="16" alt="temporary directory warning"/>


Since the text in the next line exists, there is no need for an alt text here. This icon exists only to make the notice stand out.

Resolved in 8d861dd

daniel-beck · 2024-05-24T09:56:47Z

core/src/main/resources/jenkins/model/Jenkins/oops.jelly

@@ -32,7 +32,7 @@ THE SOFTWARE.
    <l:header />
    <l:main-panel>
        <h1 style="text-align: center">
-          <img src="${imagesURL}/rage.svg" height="179" width="154"/> <span style="font-size:50px"><st:nbsp/>${%Oops!}</span>
+          <img src="${imagesURL}/rage.svg" height="179" width="154" alt=""/> <span style="font-size:50px"><st:nbsp/>${%Oops!}</span>


Inconsistent with the change in _404_simple.jelly, why?

(Personally I like this one more for the same reason as I mentioned in a number of other comments.)

Resolved in 23d56f1 by using empty alt text

Not true, this is the only line in this PR you didn't update 😉

Thanks for catching that. This was resolved in 4d8fd42

github-actions · 2024-06-18T06:35:23Z

Please take a moment and address the merge conflicts of your pull request. Thanks!

The destination page is describing the Jenkins project, not just Jenkins core. Addresses the comment in: jenkinsci#9296 (comment)

As noted by Daniel Beck, it does not help the user of the screen reader for us to show the same fundamental text message twice. The image in these cases is a hint for the reader of the message text that immediately follows the image. Cluttering the screen reader content with a second copy of the same message is not helpful.

No need to display the "not found" message twice in locations very near to each other.

MarkEWaite

I think this is ready to merge. Needs a review from at least one other maintainer

MarkEWaite · 2024-07-27T11:41:32Z

I attempted to display the memory usage monitor jelly change and could not find a way to reach it. The class is used in the system information page but the graph content is loaded JavaScript into a div on the page. The alt text in that JavaScript is "Memory usage graph" and does not appear to be internationalized.

daniel-beck · 2024-07-28T22:01:00Z

I attempted to display the memory usage monitor jelly change and could not find a way to reach it.

A possible URL for this page is /extensionList/hudson.diagnosis.MemoryUsageMonitor/0/ and it appears it has always been broken (since afddd51), because the heap/nonHeap path elements are not present in the image URL.

MarkEWaite · 2024-07-28T22:30:19Z

I attempted to display the memory usage monitor jelly change and could not find a way to reach it.

A possible URL for this page is /extensionList/hudson.diagnosis.MemoryUsageMonitor/0/ and it appears it has always been broken (since afddd51), because the heap/nonHeap path elements are not present in the image URL.

Thanks. I've opened that URL and confirmed that the updated text is displayed and that the image is not displayed

JanhviHarwani and others added 2 commits May 21, 2024 12:12

Merge pull request #1 from jenkinsci/master

7c874c4

merging current changes

Added alt attribute to all img tag

08fc4b7

Added null as text in alt attribute

4a2c910

MarkEWaite requested changes May 21, 2024

View reviewed changes

daniel-beck requested changes May 21, 2024

View reviewed changes

JanhviHarwani and others added 6 commits May 23, 2024 18:28

Merge pull request #2 from jenkinsci/master

8cf6508

merge jenkins master branch to feature/add-null-text

Merge branch 'master' of https://github.com/JanhviHarwani/jenkins int…

6d4fa24

…o feature/add-null-text-alt

Merge pull request #3 from jenkinsci/master

c7867b2

merging to local branch

Resolved all the PR comments

4d8fd42

Merge branch 'feature/add-null-text-alt' of https://github.com/Janhvi…

9eafafe

…Harwani/jenkins into feature/add-null-text-alt

Updated alt image messages for dir.jelly, _404_simple.jelly and execu…

e16257d

…tors.jelly

daniel-beck reviewed May 24, 2024

View reviewed changes

github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Jun 18, 2024

Merge branch 'master' into feature/add-null-text-alt

1b731b5

github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Jun 18, 2024

MarkEWaite added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Jun 24, 2024

MarkEWaite added 4 commits July 27, 2024 03:35

Merge branch 'master' into feature/add-null-text-alt

6275984

Do not attempt to localize Jenkins welcome page alt text

b782620

More clearly describe the participate link

87d4ab4

The destination page is describing the Jenkins project, not just Jenkins core. Addresses the comment in: jenkinsci#9296 (comment)

Internationalize memory usage monitor graph

904cb3e

MarkEWaite added 4 commits July 27, 2024 04:18

Use empty alt text in dropdown documentation example

690501b

Use empty alt text because nearby text already has the message

23d56f1

No need to display the "not found" message twice in locations very near to each other.

Use empty alt text - nearby text already provides details

f110180

MarkEWaite requested review from MarkEWaite and daniel-beck July 27, 2024 10:28

MarkEWaite changed the title ~~Feature/add null text alt~~ Feature/add null alt text to several images Jul 27, 2024

MarkEWaite approved these changes Jul 27, 2024

View reviewed changes

Merge branch 'master' into feature/add-null-text-alt

1806b45

Merge branch 'master' into feature/add-null-text-alt

42f2883

	<img width="600" alt="jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">
	<img width="600" alt="Jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

	<img width="600" alt="jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">
	<img width="600" alt="${%Jenkins welcome page}" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

	<img src="graph?type=${type}&width=500&height=300" alt="null" />
	<img src="graph?type=${type}&width=500&height=300" alt="memory usage monitor graph" />

	<img src="${imagesURL}/rage.svg" height="179" width="154" alt="null"/> <span style="font-size:50px"><st:nbsp/>${%Oops!}</span>
	<img src="${imagesURL}/rage.svg" height="179" width="154" alt="Oops"/> <span style="font-size:50px"><st:nbsp/>${%Oops!}</span>

	<img width="600" alt=${%Jenkins welcome page} src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">
	<img width="600" alt="Jenkins welcome page" src="https://raw.githubusercontent.com/jenkinsci/jenkins/master/.github/images/jenkins-welcome.svg">

Feature/add null alt text to several images #9296

Are you sure you want to change the base?

Feature/add null alt text to several images #9296

Conversation

JanhviHarwani commented May 21, 2024 • edited by MarkEWaite Loading

Testing done

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

welcome bot commented May 21, 2024

MarkEWaite commented May 21, 2024

MarkEWaite left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniel-beck May 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniel-beck May 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daniel-beck left a comment

Choose a reason for hiding this comment

daniel-beck May 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JanhviHarwani commented May 24, 2024

daniel-beck May 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarkEWaite Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarkEWaite Jul 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 18, 2024

MarkEWaite left a comment

Choose a reason for hiding this comment

MarkEWaite commented Jul 27, 2024

daniel-beck commented Jul 28, 2024

MarkEWaite commented Jul 28, 2024

JanhviHarwani commented May 21, 2024 •

edited by MarkEWaite

Loading

daniel-beck May 21, 2024 •

edited

Loading

daniel-beck May 22, 2024 •

edited

Loading

daniel-beck May 21, 2024 •

edited

Loading

daniel-beck May 24, 2024 •

edited

Loading

MarkEWaite Jul 30, 2024 •

edited

Loading

MarkEWaite Jul 30, 2024 •

edited

Loading